NETOBSERV-268 frontend code styling by jpinsonneau · Pull Request #541 · netobserv/netobserv-web-console

jpinsonneau · 2024-06-13T15:41:40Z

Description

Sort imports
Constants naming
Usage of vs css display: flex
Usage of
Define interface for props every time
Move non component files to utils (filters-helper.ts, metrics-helper.tsx)
Split netflow-traffic code that became too big for a single file => will be done in a followup
Split other files (panels / contents)

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

openshift-ci · 2024-06-13T15:41:46Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-06-13T15:45:48Z

Codecov Report

Attention: Patch coverage is 70.83812% with 254 lines in your changes missing coverage. Please review.

Project coverage is 56.43%. Comparing base (e3bdd3d) to head (1caa117).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #541      +/-   ##
==========================================
- Coverage   56.56%   56.43%   -0.14%     
==========================================
  Files         173      183      +10     
  Lines        9042     9079      +37     
  Branches     1183     1182       -1     
==========================================
+ Hits         5115     5124       +9     
- Misses       3561     3589      +28     
  Partials      366      366

Flag	Coverage Δ
uitests	`58.01% <70.83%> (+0.05%)`	⬆️
unittests	`52.03% <ø> (-0.62%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
web/src/api/loki.ts	`85.71% <100.00%> (ø)`
web/src/components/__tests-data__/filters.ts	`100.00% <100.00%> (ø)`
web/src/components/__tests-data__/flows.ts	`100.00% <100.00%> (ø)`
web/src/components/__tests-data__/metrics.ts	`100.00% <100.00%> (ø)`
web/src/components/alerts/banner.tsx	`43.75% <100.00%> (-3.31%)`	⬇️
web/src/components/alerts/fetcher.tsx	`55.00% <100.00%> (ø)`
web/src/components/dropdowns/group-dropdown.tsx	`70.58% <100.00%> (-0.85%)`	⬇️
web/src/components/dropdowns/layout-dropdown.tsx	`89.65% <100.00%> (-0.35%)`	⬇️
.../src/components/dropdowns/metric-type-dropdown.tsx	`61.76% <100.00%> (-1.10%)`	⬇️
...components/dropdowns/overview-display-dropdown.tsx	`63.63% <100.00%> (-3.04%)`	⬇️
... and 93 more

... and 2 files with indirect coverage changes

jotak

nice!
/lgtm

(edit: ah that's still a draft ... but feel free to merge and open another one if this can avoid you troubles rebasing if kept open for too long .. as you want)

jpinsonneau · 2024-06-17T13:53:21Z

nice! /lgtm

(edit: ah that's still a draft ... but feel free to merge and open another one if this can avoid you troubles rebasing if kept open for too long .. as you want)

Sorry @jotak I had other changes in my list
I think we should merge that first and I'll work on the netflow-traffic page split right after

stleerh

Just one minor comment. Everything else /lgtm.

stleerh · 2024-06-18T18:27:05Z

+    margin-top: 0 !important;
+    margin-right: 0 !important;
+    margin-bottom: 0 !important;
+    margin-left: 0 !important;


margin:0 !important; is the shortcut for lines 244-247 so you can remove lines 244-247.

sure, addressed in 1120718

jotak · 2024-06-26T08:34:26Z

@jpinsonneau LGTM, just leaving you checking the failing cypress test and Steven's comment

jpinsonneau · 2024-06-26T09:16:50Z

@jpinsonneau LGTM, just leaving you checking the failing cypress test and Steven's comment

not sure what was causing the issue. Let's see if it happens again

jpinsonneau · 2024-06-26T09:36:41Z

@memodi any idea on what could cause the cypress error ?

  netflow-topology
    ✓ displays topology and namespaces (14093ms)
    1) find network observability namespace
Warning: vkCreateInstance: Found no drivers!
Warning: vkCreateInstance failed with VK_ERROR_INCOMPATIBLE_DRIVER
    at CheckVkSuccessImpl (../../third_party/dawn/src/dawn/native/vulkan/VulkanError.cpp:88)
    at CreateVkInstance (../../third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:458)
    at Initialize (../../third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:344)
    at Create (../../third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:266)
    at operator() (../../third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:521)
    ✓ update options (62983ms)
  2 passing (2m)
  1 failing
  1) netflow-topology
       find network observability namespace:
     AssertionError: Timed out retrying after 10000ms: Expected to find element: `.pf-c-chart`, but never found it.
      at Context.eval (webpack://netobserv-plugin/./cypress/e2e/topology/topology.spec.ts:41:26)

jpinsonneau · 2024-06-26T13:57:42Z

Seems like 1caa117 fixed it 😸

memodi · 2024-06-26T15:47:42Z

Seems like 1caa117 fixed it 😸

glad, you were able to figure this out. I'd not have idea. Perhaps we should enable video recording and archiving images from failed runs to the build logs?

jotak · 2024-06-26T15:50:02Z

/lgtm

openshift-ci · 2024-06-27T07:28:52Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

memodi · 2024-06-27T15:29:38Z

cc @Amoghrd this has been merged, were there any regression test impact?

Amoghrd · 2024-06-27T15:41:08Z

Havent tested it. Will run regression. Was waiting for a reply on JIRA

Amoghrd · 2024-06-27T18:27:20Z

Regression is passing cypress-tests#664 with some flakes. All passed on manual rerun. But table_queryopts and netflow_table tests required changes; Made here: openshift-tests-private#17547

* prettier sort imports * sort imports + constants naming * flex + flexitem * TextContent and Text usage * interfaces for props * move helpers + voronoi as component * split dropdowns / panels when possible * fixes * removed unecessary CSS * fix voronoi container

jpinsonneau added 2 commits June 13, 2024 17:40

prettier sort imports

d48924a

sort imports + constants naming

3153f78

openshift-ci Bot added the do-not-merge/work-in-progress label Jun 13, 2024

jotak approved these changes Jun 14, 2024

View reviewed changes

openshift-ci Bot assigned jotak Jun 14, 2024

openshift-ci Bot added the lgtm label Jun 14, 2024

jpinsonneau added 2 commits June 14, 2024 10:42

flex + flexitem

dead892

TextContent and Text usage

2d2da9a

openshift-ci Bot removed the lgtm label Jun 17, 2024

jpinsonneau added 4 commits June 17, 2024 13:01

interfaces for props

95713bb

move helpers + voronoi as component

58868cc

split dropdowns / panels when possible

c320bcf

fixes

78fb073

jpinsonneau marked this pull request as ready for review June 17, 2024 13:52

openshift-ci Bot removed the do-not-merge/work-in-progress label Jun 17, 2024

stleerh reviewed Jun 18, 2024

View reviewed changes

removed unecessary CSS

1120718

fix voronoi container

1caa117

openshift-ci Bot added the lgtm label Jun 26, 2024

jpinsonneau added the approved label Jun 27, 2024

openshift-merge-bot Bot merged commit 56ada60 into netobserv:main Jun 27, 2024

jpinsonneau mentioned this pull request Jun 27, 2024

NETOBSERV-1764 frontend code styling followup #551

Merged

10 tasks

jotak mentioned this pull request Jul 23, 2025

NETOBSERV-1942 config / status views + NETOBSERV-1945 resource calculator #763

Merged

10 tasks

Conversation

jpinsonneau commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Checklist

Uh oh!

openshift-ci Bot commented Jun 13, 2024

Uh oh!

codecov Bot commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jotak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpinsonneau commented Jun 17, 2024

Uh oh!

stleerh left a comment

Choose a reason for hiding this comment

Uh oh!

stleerh Jun 18, 2024

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

jotak commented Jun 26, 2024

Uh oh!

jpinsonneau commented Jun 26, 2024

Uh oh!

jpinsonneau commented Jun 26, 2024

Uh oh!

jpinsonneau commented Jun 26, 2024

Uh oh!

memodi commented Jun 26, 2024

Uh oh!

jotak commented Jun 26, 2024

Uh oh!

openshift-ci Bot commented Jun 27, 2024

Uh oh!

memodi commented Jun 27, 2024

Uh oh!

Amoghrd commented Jun 27, 2024

Uh oh!

Amoghrd commented Jun 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jpinsonneau commented Jun 13, 2024 •

edited

Loading

codecov Bot commented Jun 13, 2024 •

edited

Loading

jotak left a comment •

edited

Loading